Skip to content

[superlog] Resolve domain names to UUID before creating annotations#475

Open
superlog-app[bot] wants to merge 1 commit into
stagingfrom
superlog/fix-annotations-domain-to-uuid
Open

[superlog] Resolve domain names to UUID before creating annotations#475
superlog-app[bot] wants to merge 1 commit into
stagingfrom
superlog/fix-annotations-domain-to-uuid

Conversation

@superlog-app

@superlog-app superlog-app Bot commented Jun 13, 2026

Copy link
Copy Markdown

Summary

The insights agent's create_annotation (and list_annotations) tool failed with NOT_FOUND: website not found whenever the AI model passed a domain name (e.g. app.quiver.ai) instead of a UUID as websiteId. This happened consistently because the model observes the domain being used successfully as websiteId in ClickHouse SQL queries — and reused it verbatim when calling the annotation tool.

The annotation tool passed the raw AI-supplied string directly to the RPC, which does a PostgreSQL UUID-only lookup. Since domain names never match, every annotation creation attempt silently failed across all scheduled insights runs.

The fix has two parts:

  1. resolveToolWebsite (utils/context.ts) — extends the existing resolver to also match by domain name against the accessible-websites list, and by ctx.websiteDomain for the single-site insights context.
  2. annotations.ts — wires resolveToolWebsite into both list_annotations and create_annotation, so the resolved UUID is used for all RPC calls.

An alternative approach would be to add a domain→UUID lookup server-side in the RPC annotations.create handler. That would be more defensive but silently accepts incorrect input; the client-side resolution keeps the tool consistent with execute_sql and web_metrics and catches misconfigured contexts early.

Incident on Superlog


Was this PR helpful? Leave feedback — goes straight to the Superlog team.


Summary by cubic

Resolve domain names to UUIDs before calling annotation RPCs to stop "website not found" errors when a domain is used as websiteId. Annotations can now be listed and created reliably in scheduled insights.

  • Bug Fixes
    • Extended resolveToolWebsite to map domains to UUIDs via accessible websites or ctx.websiteDomain.
    • Updated list_annotations and create_annotation to use the resolved UUID for RPC calls.
    • Added tests for domain resolution and error handling.

Written for commit edfeeda. Summary will update on new commits.

Review in cubic

@vercel

vercel Bot commented Jun 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
databuddy-status Ready Ready Preview, Comment Jun 13, 2026 9:20am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
dashboard Skipped Skipped Jun 13, 2026 9:20am
documentation Skipped Skipped Jun 13, 2026 9:20am

@unkey-deploy

unkey-deploy Bot commented Jun 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Unkey Deploy

Name Status Preview Inspect Updated (UTC)
api (preview) Ready Visit Preview Inspect Jun 13, 2026 9:20am

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes annotation tool failures where the AI model passed a domain name (e.g. app.quiver.ai) instead of a UUID as websiteId, causing every RPC call to return NOT_FOUND. The resolver now has a domain-name slow-path that maps the supplied string to the correct UUID before any RPC is made.

  • context.tsresolveToolWebsite gains a two-step slow-path: first scans accessibleWebsites for a domain match, then falls back to ctx.websiteDomain for single-site insight contexts.
  • annotations.ts — both list_annotations and create_annotation now call resolveToolWebsite and forward the resolved UUID; update_annotation and delete_annotation are unaffected as they don't accept a websiteId.
  • context.test.ts — three new unit tests cover domain→UUID resolution, context-domain fallback, and the reject-unknown-domain error path.

Confidence Score: 3/5

The core fix is correct and well-tested, but the domain comparison in resolveToolWebsite is case-sensitive; a mixed-case domain from the model would bypass the new slow-path and re-produce the original NOT_FOUND error.

The resolver's domain lookup uses strict === equality. Domain names are case-insensitive by RFC 1035, and the AI model may reproduce the domain with any casing it observes in query results. If the stored domain is app.quiver.ai and the model passes App.Quiver.AI, the slow-path miss silently re-throws, leaving the annotation tools broken for that casing variant — the same symptom this PR was written to fix.

packages/ai/src/ai/tools/utils/context.ts — the domain comparison and the fallback ctx.websiteDomain check both need case-normalisation before the fix is unconditionally reliable.

Important Files Changed

Filename Overview
packages/ai/src/ai/tools/utils/context.ts Adds domain-name slow-path to resolveToolWebsite; case-sensitive comparison risks the fix silently failing for mixed-case domains
packages/ai/src/ai/tools/annotations.ts Wires resolveToolWebsite into list_annotations and create_annotation; integration is straightforward and correct
packages/ai/src/ai/tools/utils/context.test.ts Adds three targeted test cases covering domain→UUID resolution; all scenarios are correct and mirror the new code paths

Sequence Diagram

sequenceDiagram
    participant AI as AI Model
    participant AT as annotations.ts
    participant RW as resolveToolWebsite
    participant RPC as RPC (annotations.create / list)

    AI->>AT: "websiteId = "app.quiver.ai""
    AT->>RW: resolveToolWebsite(ctx, "app.quiver.ai")
    Note over RW: Fast path: UUID match in accessibleWebsites or ctx.websiteId?
    RW-->>RW: No UUID match found
    Note over RW: Slow path: domain match in accessibleWebsites?
    RW-->>RW: "w.domain === "app.quiver.ai" → found web_uuid_123"
    RW-->>AT: "{ websiteId: "web_uuid_123", domain: "app.quiver.ai" }"
    AT->>RPC: "websiteId = "web_uuid_123""
    RPC-->>AT: success
    AT-->>AI: annotation created
Loading

Reviews (1): Last reviewed commit: "[superlog] Resolve domain names to UUID ..." | Re-trigger Greptile

Comment on lines +40 to +46
const byDomain = accessible.find(
(w) => w.domain != null && w.domain === inputWebsiteId
);
if (byDomain) {
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };
}
if (inputWebsiteId === ctx.websiteDomain && ctx.websiteId) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Domain matching is case-sensitive via ===, but RFC 1035 treats hostnames as case-insensitive. If the AI model passes a domain with different casing than what is stored (e.g. App.Quiver.AI vs app.quiver.ai), the slow-path lookup silently falls through to the error throw, leaving the original bug intact for that casing variant. Normalising both sides to lowercase costs nothing and makes the fix robust.

Suggested change
const byDomain = accessible.find(
(w) => w.domain != null && w.domain === inputWebsiteId
);
if (byDomain) {
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };
}
if (inputWebsiteId === ctx.websiteDomain && ctx.websiteId) {
const normalised = inputWebsiteId.toLowerCase();
const byDomain = accessible.find(
(w) => w.domain != null && w.domain.toLowerCase() === normalised
);
if (byDomain) {
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };
}
if (ctx.websiteDomain?.toLowerCase() === normalised && ctx.websiteId) {

(w) => w.domain != null && w.domain === inputWebsiteId
);
if (byDomain) {
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The ?? undefined is dead code here: the find predicate already guards w.domain != null, so byDomain.domain is always a non-null string at this point. The nullish coalescing just adds noise.

Suggested change
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };
return { websiteId: byDomain.id, domain: byDomain.domain };

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants